Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Context for cancellation and timeouts #9

Merged
merged 4 commits into from
Oct 18, 2022
Merged

Conversation

hryx
Copy link
Contributor

@hryx hryx commented Oct 18, 2022

This includes preparatory work from #7 that:

  • Uses the stdlib Context to control job cancellation, abort on client disconnection, and set timeouts for tasks
  • Adds timeouts to Config
  • Fixes a couple uncaught failures
  • Adds some GCS setup info to the readme

Once merged I will rebase #7 on master.

Breaking library changes

In the zipserver package:

  • All methods of the Storage interface now take a Context argument.
  • Archiver.ExtractZip() and Archiver.UploadZipFromFile() now take a Context argument.

hryx added 4 commits October 16, 2022 14:58
- Observe the response code from GCS in case of non-OK result.
  This is separate from the error value returned by client.Do().
- Use Context to cancel task dispatch. This handles multiple/concurrent
  cancelations idempotently.
It is recommended to set timeouts on all outgoing requests in order
to prevent indefinitely open connections or other dangling resources.
The way to do that is with the stdlib Context, which conventionally
occupies the first function parameter.

This change uses Context in two ways:

1. Propagating Context of an incoming HTTP request in the handler
   (this is provided by the Go HTTP server per request).
2. Adding ctx params to the Storage interface methods and passing
   inherited Contexts to them.

Observing Contexts provided to handlers also means we can abort work
early if the client has canceled the request.

As the http package was stabilized before Context was introduced,
most HTTP calls have to be made with http.NewRequestWithContext
instead of the convenient http.Get, http.PostForm, etc.

The asynchronous operations are given Contexts detached from the
original parent; otherwise, the request would end and the workload
would be canceled.

I added some timeout fields to Config with default values (we can
change these durations as appropriate for the workloads). As an
oversight in the Go standard library, time.Duration does not natively
support JSON marhaling/unmarhaling, so I had to make a small wrapper
type for these.
@leafo leafo merged commit 15f26f6 into itchio:master Oct 18, 2022
@leafo
Copy link
Member

leafo commented Oct 18, 2022

Looks good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants